Skip to content

Conversation

@andreasasprou
Copy link
Collaborator

Summary

Implements workspace status indicators showing agent lifecycle states:

  • Amber (pulsing): Agent actively processing
  • Red (pulsing): Agent blocked, needs user input
  • Green (static): Agent completed, ready for review

Stack

⚠️ This PR is stacked on #559 (workspace-sidebar) and should be merged after that PR.

Changes

Workspace Status Indicators

  • PaneStatus enum: "idle" | "working" | "permission" | "review"
  • Status aggregation: workspace shows highest-priority status across all panes (permission > working > review)
  • Click behavior:
    • reviewidle (acknowledged)
    • permissionworking (assumes permission granted)
    • working → unchanged (persists until Stop event)
  • App restart: stale "working" status cleared on startup
  • Migration: old needsAttention boolean migrated to status: "review"

Agent Hook Integration

  • Claude Code: UserPromptSubmit → Start, Stop → Stop, PermissionRequest → Permission
  • OpenCode: session.status.busy → Start, session.status.idle → Stop, permission.ask → Permission
  • Codex: partial support (review only, no working indicator)

Dev/Prod Separation (Hardening)

  • Removed global OpenCode plugin write (was causing cross-talk between dev/prod)
  • Added startup cleanup for stale global plugins
  • Server ignores unknown event types (forward compatibility)
  • notify.sh no longer defaults to "Stop" on parse failure
  • Added SUPERSET_ENV and SUPERSET_HOOK_VERSION to terminal environment
  • Server validates environment and logs mismatches

UI Locations Updated

  • Top bar workspace tabs (WorkspaceItem.tsx)
  • Sidebar workspace list (WorkspaceListItem.tsx)
  • Group strip tabs (GroupStrip.tsx)
  • Tab item in sidebar (TabItem/index.tsx)

Testing

  • Manual QA completed for Claude Code and OpenCode
  • Unit tests added for mapEventType() and terminal env vars

Breaking Changes

None - backwards compatible with existing persisted state (migration handled automatically)

andreasasprou and others added 30 commits January 2, 2026 08:07
Introduces a workspace-level view mode toggle allowing users to switch between:
- **Workbench mode**: Mosaic panes layout with terminals + file viewers for in-flow work
- **Review mode**: Dedicated Changes page for focused code review

Key changes:
- Add ViewModeToggle component in workspace header (prominent segmented control)
- Add FileViewerPane with Raw/Rendered/Diff modes, lock/unlock, and split support
- Add GroupStrip for group switching above Mosaic content
- Unify sidebar to use full ChangesView in both modes (with onFileOpen callback)
- Add workspace-view-mode store with per-workspace persistence
- Add readWorkingFile tRPC procedure for safe file reads (size/binary checks)
- Wire file clicks to open/reuse FileViewer panes (MRU unlocked policy)
- Cmd+T in Review mode switches to Workbench first, then creates terminal
- Add !!worktreePath checks to FileViewerPane query enabled conditions
- Add !!worktreePath checks to save handler guards (handleSaveRaw, handleSaveDiff)
- Fix stale state reference after addTab() in addFileViewerPane action

Addresses CodeRabbit review feedback.
… and UX improvements

- Add validatePathForWrite() to prevent path traversal attacks in saveFile
- Add aria-pressed attribute to ViewModeToggle buttons for accessibility
- Increase close button touch target in GroupStrip for better UX
- Add .mdx file support for rendered view mode
P0 Security:
- Add path validation to getUnstagedVersions before readFile (prevents traversal)
- Key Editor/DiffViewer by filePath to force remount (fixes stale Cmd+S closure)

P1 Fixes:
- Update originalContentRef when raw content loads (dirty tracking fix)
- Add .mdx to isMarkdown check for toolbar consistency
- Gate diff editable to staged/unstaged only (against-main/committed now read-only)

P2 Performance:
- Add safeGitShow helper with 2MB size limit on all git show calls
- Add size check before reading working tree files in getUnstagedVersions
- P0-1: Add file-viewer type and fileViewer object to paneSchema for tab persistence
- P0-2: Fix symlink escape vulnerability in validatePathForWrite by checking
  if target is symlink and resolving parent directory paths
- P1-1: Pass defaultBranch to FileViewerPane for against-main diffs
- P1-2: Switch staged diff to unstaged after save (matches Review mode behavior)
- P2: Use Buffer.byteLength instead of string.length in safeGitShow for
  accurate UTF-8 byte counting
P0 (critical):
- Remove duplicate saveFile from git-operations.ts that was overwriting
  the hardened version in file-contents.ts (security vulnerability)

P1 (must fix):
- Use basename()/dirname() from node:path instead of split('/') for
  cross-platform Windows compatibility in validatePathForWrite
- Add preflight size check with git cat-file -s in safeGitShow to
  prevent memory spikes from large blobs before materializing content

P2 (UX):
- Fix split pane to clone file-viewer state instead of creating terminal
  when splitting a file-viewer pane (locked by default)

Question fix:
- Show GroupStrip with add button even when tabs.length === 0 so users
  have visible UI to create new terminal (not just hotkey)
…ocalDb

P0 (CRITICAL SECURITY):
- Add validateWorktreePathInDb() that verifies worktreePath exists in
  localDb.worktrees before any filesystem operations
- Without this, a compromised renderer could read/write arbitrary files
  by passing worktreePath='/' and filePath='.ssh/id_rsa'
- Applied to getFileContents, saveFile, and readWorkingFile procedures

P1 (correctness):
- Replace startsWith('..') checks with segment-aware containsPathTraversal()
  and isPathOutsideBase() helpers that use path.sep
- Fixes false positives on valid paths like '..foo/bar' (directories
  starting with '..')
- Cross-platform compatible (handles both / and \ separators)

P2 (performance):
- Guard killTerminalForPane() calls on pane.type === 'terminal'
- Prevents unnecessary IPC and warning logs when closing file-viewer panes
…ve editor drafts

P0 (security):
- Extract assertWorktreePathInDb to shared security.ts
- Returns worktree record to avoid duplicate queries
- Apply validation to ALL routes: git-operations, status, staging, branches

P1 (data loss):
- Preserve unsaved editor content across view mode switches
- Store draft in ref before switching away from raw mode
- Restore draft when returning to raw mode
- Clear draft on save and file change
- Update dirty state on editor mount with draft content
…otection

P0 Security (BLOCK):
- Add validatePathInWorktree() check before rm() in deleteUntracked
- Prevents path traversal (../) and symlink escape attacks
- Consolidated path validation utilities in security.ts

P1 Data Loss:
- Track save source (Raw vs Diff) with savingFromRawRef
- Only clear draft when saving from Raw mode
- Disable Diff editing when Raw draft exists (forces user to save/discard)

P2:
- Use ['--', filePath] in git.add() to handle paths starting with -
…th validation

- Reduce security.ts from 213 to 65 lines
- Remove async symlink/realpath checking (users own their repos)
- Remove segment-aware path traversal (..foo edge case not worth complexity)
- Keep worktreePath DB validation (the real security boundary)
- Keep simple .. and absolute path checks (sufficient for 99.99% of cases)
P0 (blocking):
- Add path validation (rejects .. and absolute) to applyUntrackedLineCount
- Add 1MB size cap to prevent OOM on large untracked files during polling

P2:
- Add type guard in updateTabLayout - only call killTerminalForPane for terminal panes
- Use ['--', branch] in switchBranch to ensure branch treated as refname
…lidation

- Add path-validation.ts with industry-standard containment check (path.relative)
- Add secure-fs.ts with self-validating FS wrappers (symlink escape protection)
- Add git-commands.ts with semantic helpers (gitSwitchBranch vs gitCheckoutFile)
- Fix switchBranch: use 'git switch' instead of 'git checkout --' (was file checkout)
- Fix path traversal check: segment-aware (allows ..foo, rejects ..)
- Fix symlink bypass: check parent dirs for new files, use stat not lstat
- Fix worktree root deletion: explicit allowRoot check
- All FS operations now go through secureFs with validation built-in
Add a setting to control whether Cmd+clicking file paths in the terminal
opens them in an external editor (default) or in the in-app FileViewerPane.

- Add terminalLinkBehavior setting to local-db schema with migration
- Add getTerminalLinkBehavior/setTerminalLinkBehavior tRPC procedures
- Refactor createTerminalInstance to accept onFileLinkClick callback
- Wire up setting in Terminal.tsx with ref pattern (avoids terminal recreation)
- Add Select UI in BehaviorSettings for choosing link behavior
Remove async symlink escape detection from path validation since the
threat model doesn't justify it: a compromised renderer already has
terminal access for arbitrary command execution.

Changes:
- Replace resolveSecurePath (async) with resolvePathInWorktree (sync)
- Remove assertNoSymlinkEscape and checkSymlinks option
- Add validateRelativePath for simple path safety checks
- Update secure-fs.ts to use new sync functions
- Update threat model documentation in path-validation.ts
…s sidebar)

Add a setting to let users choose between displaying workspaces as
horizontal tabs in the TopBar (current behavior) or in a dedicated left
sidebar (new feature, similar to Linear/GitHub Desktop).

Key changes:
- Add navigationStyle column to settings table (migration 0005)
- Add navigation style dropdown in Behavior Settings
- Create WorkspaceSidebar component with collapsible project sections
- Create shared useWorkspaceShortcuts hook (⌘1-9 shortcuts, auto-create)
- Update TopBar to conditionally render based on navigation style
- Add ⌘⇧B hotkey to toggle workspace sidebar

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
…ition

P0: Add CreateWorkspaceButton to TopBar when in sidebar mode
- Button was previously only rendered via WorkspaceTabs
- Now renders in top-right section when navigationStyle is sidebar

P1: Fix race condition in createBranchWorkspace
- Add unique partial index on (projectId) WHERE type='branch'
- Use INSERT ON CONFLICT DO NOTHING to handle concurrent calls
- If conflict, fetch the existing workspace instead of failing

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
WorkspaceListItem now has full feature parity:
- Close/delete button with confirmation dialog
- Context menu (Rename, Open in Finder)
- Hover card with PR details, checks, reviews
- Needs attention indicator (red pulse)
- Inline rename (double-click)
- Drag & drop reordering
- BranchSwitcher for branch workspaces

Other changes:
- Remove CreateWorkspaceButton from TopBar in sidebar mode
- Add per-project "Add workspace" dropdown (New Workspace, Quick Create)
- Add preSelectedProjectId to modal store for pre-selecting project
- Simplify GroupStrip to use consistent browser-tab style

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Extract useWorkspaceDeleteHandler hook for shared delete logic
- Use existing extractPaneIdsFromLayout from tabs/utils instead of inline collectPaneIds
- Add named constants for magic numbers (staleTime, delays, shortcut index)
- Reduces code duplication between WorkspaceItem and WorkspaceListItem

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
…ar mode

Move SidebarControl to shared location (screens/main/components/) and
conditionally render based on navigation style:
- top-bar mode: toggle in TopBar (unchanged)
- sidebar mode: toggle in WorkspaceActionBar (left side, before branch info)

This improves UX by placing the sidebar toggle closer to the content
it controls when in sidebar navigation mode.
Default to Mac layout (80px padding) while platform query is loading,
since undefined === 'darwin' evaluates to false, causing 16px padding
to be used initially on Mac before the query resolves.
…minal links

P0: Fix branch workspace support in assertRegisteredWorktree
- Extended validation to check both worktrees.path AND projects.mainRepoPath
- Branch workspaces use mainRepoPath which wasn't being validated

P1: Fix terminal file-viewer links for absolute paths
- Normalize absolute paths to worktree-relative before opening file viewer
- File viewer expects relative paths but terminal links can be absolute

P2: Fix misleading security comments
- Removed claims about symlink checks that aren't implemented
- Comments now accurately describe worktree registration + path traversal validation
- Add markWorkspaceAsUnread action to tabs store that sets needsAttention=true
  for all panes in a workspace
- Add context menu item with LuEyeOff icon to:
  - WorkspaceItemContextMenu (top bar tabs)
  - WorkspaceListItem (sidebar) for both branch and worktree workspaces
- Leverages existing needsAttention indicator system (red pulsing dot)
- Logs when no panes exist in workspace (empty workspace edge case)
Previously, clicking a workspace only set it as active but didn't clear
the needsAttention state on panes. This meant 'Mark as Unread' worked
but clicking the workspace didn't mark it as read.

Added clearWorkspaceAttention action that clears needsAttention for all
panes in a workspace, called when clicking workspace in both top bar
and sidebar.
P0: Make unique branch workspace migration resilient to existing duplicates
- Dedupe existing duplicate branch workspaces before creating unique index
- Update settings.last_active_workspace_id if it points to deleted workspace

P1: Fix isResizing persistence and store selector usage
- Exclude ephemeral isResizing from Zustand persistence via partialize
- Use selector pattern in MainScreen to avoid unnecessary re-renders

P1: Fix delete handler to show dialog when canDeleteData is undefined
- Safe default: show confirmation when query fails

P2: Fix path boundary check in terminal file links
- Use path.startsWith(cwd + '/') to avoid false prefix matches

P2: Add missing drizzle migration snapshot
- Created 0006_snapshot.json for consistency
P0: Add symlink escape protection for File Viewer writes
- Block writes if file's realpath escapes worktree boundary
- Add isSymlinkEscaping() method for UI to detect and warn users
- Update threat model docs to reflect new symlink protection

P1: Fix race condition in createBranchWorkspace ordering
- Insert workspace first, then shift others only on success
- Losers of the race no longer corrupt tabOrder
- Exclude newly inserted workspace from shift operation

P2: Fix migration to keep most recently used duplicate
- Select survivor by last_opened_at DESC (not arbitrary MIN(id))
- Use id ASC as tiebreaker when last_opened_at is equal
Consolidate two navigation bars into a single unified top bar:

- Create WorkspaceControls component group in TopBar/
  - BranchIndicator: shows current branch (keyboard-focusable)
  - OpenInMenuButton: compact Open button with dropdown
  - ViewModeToggleCompact: Workbench/Review toggle (24px height)

- Remove WorkspaceActionBar from WorkspaceView entirely
- Delete unused WorkspaceActionBar component folder

Improvements based on Oracle review:
- Optimize Zustand selector to minimize rerenders
- Add toast error handling for open/copy mutations
- Add path to Open button tooltip for discoverability
- Add focus-visible rings for keyboard accessibility
- Use text-xs for consistent typography

Also includes minor lint auto-fixes (import sorting, template literals)
- Simplify useWorkspaceDeleteHandler to always show dialog instead of
  auto-deleting clean workspaces
- Update tooltip from 'Delete workspace' to 'Close or delete' for clarity
- Add isUnread state for workspace unread tracking
- Add workspace setUnread mutation and schema migration
- Fix hotkey() typo to defineHotkey() in hotkeys.ts
- Add getHotkey() helper that returns string (not null) for useHotkeys compatibility
- Replace all HOTKEYS.*.keys usages with getHotkey() calls
- Fix 'against-main' to 'against-base' in ChangeCategory references
- Add proper type annotation in WorkspaceSidebarControl.tsx
andreasasprou and others added 25 commits January 3, 2026 20:10
- Run PTYs in per-session subprocesses and frame IO as binary
- Time-slice headless emulator output processing to keep daemon responsive
- Handle PTY input backpressure (EAGAIN/EWOULDBLOCK) without dropping chunks
- Improve renderer paste handling (bracketed paste + chunking) and surface errors
- Track async GPU renderer via ref to reliably clear WebGL atlas
- Schedule fit+refresh on focus/resize to avoid partial renders when switching panes
Avoid fit/PTY resizes and WebGL atlas clears on focus; do a lightweight refresh instead.
- Remove duplicate terminalError handler in daemon-manager.ts
  (was causing duplicate event emissions)

- Fix failing write test in manager.test.ts
  (add async wait for PtyWriteQueue flush before assertion)

- Fix attach() hang with continuous output in session.ts
  (add 500ms timeout to flushEmulatorWrites to prevent indefinite
  hang when processes like tail -f produce continuous output)

- Apply biome formatting fixes
Retry focus redraw until container has non-zero size to avoid WebGL glitches on tab switches.
- Default xterm renderer to Canvas on macOS to avoid WebGL corruption on tab switches
- Remove focus redraw hacks that were amplifying WebGL glitches
- Fix initialCommands race: use waitForReady() instead of setTimeout(100)
- Add PTY ready promise in session to signal when subprocess spawned
- Add queue limits for both subprocess stdin and client notify queues
- Emit terminalError when writeNoAck drops input due to full queue
- Complete detachAllListeners: add disconnect: and error: prefixes
- Shutdown orphaned daemon when persistence disabled on app startup
- Extract magic number to SESSION_CLEANUP_DELAY_MS constant in daemon-manager
- Move planning docs to docs/ directory
- Extract useTerminalConnection hook to encapsulate tRPC mutations and refs
- Refactor Terminal.tsx to use the new hook, reducing component complexity
…tence disabled

Previously, shutdownOrphanedDaemon() would call client.shutdown() which
internally calls ensureConnected(), spawning a new daemon just to
immediately shut it down. This happened on every app startup when
terminal persistence was disabled.

Added tryConnectAndAuthenticate() and shutdownIfRunning() methods to
TerminalHostClient that only attempt cleanup if a daemon is already
running, avoiding wasteful spawn+kill cycles.
- Reframe PtyWriteQueue docstring to accurately describe limitations
  (reduces event loop starvation, does not prevent blocking)
- Rename prototype/ to __tests__/ for conventional test organization
For TUI sessions (alternate screen mode), serialized snapshots render
incorrectly due to styled spaces and positioning issues. Instead of
trying to perfectly serialize and restore TUI state, we now:

1. Skip writing the broken snapshot for alt-screen sessions
2. Enter alt-screen mode directly
3. Enable streaming first so live PTY output comes through
4. Trigger SIGWINCH via resize down/up - TUI redraws itself

Trade-off: Brief visual flash as TUI redraws, but the result is correct.
Normal shell sessions still use the snapshot approach which works well.

- Add SIGWINCH-based redraw for TUI (alt-screen) session reattach
- Remove dead resize nudge code (now handled by SIGWINCH approach)
- Clean up verbose debug logging from investigation
- Update research doc with final fix documentation
- Add snapshot boundary tracking for consistent daemon-side captures
…allback

The fallback logic for detecting alternate screen mode was using
presence/absence checks (includes) instead of position comparison
(lastIndexOf). This caused incorrect detection when a user entered
and exited alternate screen multiple times (e.g., opened vim, closed
it, opened it again).

Changed to use lastIndexOf comparison, matching the pattern already
used in updateModesFromData and for bracketed paste detection.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Merge 3 separate documentation files into a single comprehensive reference:
- TERMINAL_RENDERING_REATTACH_RESEARCH.md (research log)
- 20251229-terminal-host-daemon-terminal-persistence.md (exec plan)
- LARGE_PASTE_HANG_ANALYSIS.md (bug analysis)

New file uses date prefix for chronological sorting.
- Add escalation watchdog in handleKill: SIGTERM → SIGKILL → force exit
  node-pty's onExit callback doesn't fire reliably after pty.kill(SIGTERM)
- Fix dispose() async bug: capture subprocess ref before nullifying
- Add diagnostic logging throughout kill flow for debugging
- Fix Terminal.tsx hook dependency warnings with targeted biome-ignore
- Add TERMINAL_HOST_RUNBOOK.md for daemon debugging/testing
Keep essential warnings (force exit, attach timeout, force dispose stuck session).
Remove step-by-step debugging logs that are too noisy for production.
…kspace switch

When terminal persistence is enabled, render all tabs from all workspaces
and use visibility:hidden for inactive ones. This eliminates the
unmount/remount cycle that caused race conditions during TUI reattach.

Changes:
- TabsContent: query terminalPersistence setting, render all tabs when enabled
- TerminalSettings: add memory warning copy
- Terminal.tsx: remove debug logging, add comments clarifying SIGWINCH as fallback
- Technical notes: document the approach and trade-offs
In daemon mode, both scrollback and snapshot.snapshotAnsi contained
identical ANSI content, doubling IPC payload size (~500KB → 1MB).

Changes:
- daemon-manager: set scrollback to empty string in daemon mode
- Terminal.tsx: use initialAnsi variable preferring snapshot.snapshotAnsi
- Terminal.tsx: use snapshot.cwd directly instead of parsing ANSI
- Terminal.tsx: only run escape scanning when snapshot.modes unavailable
- types.ts: add JSDoc clarifying daemon mode behavior

~50% reduction in IPC payload for terminal sessions.
- Remove unused ptyPid property from Session
- Remove unused flushEmulatorWrites method (superseded by flushEmulatorWritesUpTo)
- Remove unused getSession method (superseded by getActiveSession)
- Fix template literal style in Terminal.tsx
- Fix export ordering in hooks/index.ts
Implements workspace status indicators showing agent lifecycle states:
- Amber (pulsing): Agent actively processing
- Red (pulsing): Agent blocked, needs user input
- Green (static): Agent completed, ready for review

Key features:
- Status aggregation: workspace shows highest-priority status across all panes
- Click behavior: review → idle, permission → working, working unchanged
- App restart: stale 'working' status cleared on startup
- Migration: old needsAttention boolean migrated to status enum

Dev/prod separation hardening:
- Removed global OpenCode plugin write (was causing cross-talk)
- Added startup cleanup for stale global plugins
- Server ignores unknown event types (forward compatibility)
- notify.sh no longer defaults to 'Stop' on parse failure
- Added SUPERSET_ENV and SUPERSET_HOOK_VERSION to terminal environment
- Server validates environment and logs mismatches
…atus

P0 Fixes:
- Guard setPaneStatus against unknown paneId (prevents store corruption)
- Guard markPaneAsUsed, updatePaneCwd, clearPaneInitialData (same pattern)

P1 Fix:
- Add status field to paneSchema in ui-state router (enables persistence)
- Remove obsolete needsAttention field from schema

P2 Fix:
- Validate paneId exists in resolvePaneId before returning
- Fall through to tabId/workspaceId resolution for stale paneIds
… value

Addresses code review feedback P2-A: single source of truth for protocol version.
These are temporary session artifacts that should not ship in the PR.
- Create StatusIndicator component with lookup-based config
- Export getStatusTooltip() for consumers needing tooltips
- Update 4 consumers: WorkspaceItem, WorkspaceListItem, TabItem, GroupStrip
- Remove ~67 lines of duplicated JSX across components
- Follows AGENTS.md: lookup objects over conditionals
andreasasprou added a commit to andreasasprou/superset that referenced this pull request Jan 3, 2026
Includes:
- feat(desktop): add 3-color workspace status indicators
- feat(desktop): terminal persistence via daemon process (stacked from superset-sh#541)
- refactor(desktop): extract StatusIndicator shared component
- Multiple security and UX fixes

Conflicts resolved:
- apps/desktop/src/main/terminal-host/session.ts: duplicate ptyPid property declaration
@andreasasprou
Copy link
Collaborator Author

Superseded by #588 which properly targets workspace-sidebar branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant